Skip to content

Conversation

@crbelaus
Copy link
Contributor

@crbelaus crbelaus commented Jun 8, 2024

This pull request allows us to set relevant context for the current process and also includes a plug that builds up the context for the current request.

The new ErrorTracker.put_context/1 function uses the process dictionary to store the given context. The context is additive which means that we can call ErrorTracker.put_context/1 multiple times and each call will append to the previous context. If we set twice the same key, the last value will override the previous ones.

There is also a new ErrorTracker.Integrations.Plug.RequestContext (which may need a better name) that automatically populates the context with relevant data for the current request such as: method, path, query string, headers, etc.

The Oban integration has been updated to set the context when the job starts instead of only during a captured exception.


I made the choice of using dots to namespace the context keys. For example we have "request.host" and "request.method" instead of "request" => %{"host" => "", "method" => ""}.

I think that having plain keys by default makes them easier to override by clients if they want to. To override an existing key or add a new one to an existing namespace you just add that key and call it a day. If we used the nested map approach you would need to handle deeply nested keys which makes it more difficult.

In the UI we can use the dot as namespace separator and group the keys as we please. I am still not 100% sure about this so I am open to suggestions.

@crbelaus crbelaus requested a review from odarriba June 8, 2024 11:31
@crbelaus crbelaus changed the title [WIP] Error context Error context Jun 8, 2024
@crbelaus crbelaus self-assigned this Jun 8, 2024
@crbelaus crbelaus changed the base branch from main to improve-plug-integration June 10, 2024 16:58
Base automatically changed from improve-plug-integration to main June 10, 2024 19:35
Copy link
Contributor

@odarriba odarriba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added some comments. It is mostly ready, the only part I don't know is getting the right context on Phoenix/Plug errors due to when the context is populated.

@crbelaus crbelaus merged commit 7bc8c11 into main Jun 18, 2024
@crbelaus crbelaus deleted the error-context branch June 18, 2024 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants